Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

webfonts addon #5178

Open
wants to merge 14 commits into
base: master
Choose a base branch
from
Open

webfonts addon #5178

wants to merge 14 commits into from

Conversation

jerch
Copy link
Member

@jerch jerch commented Oct 3, 2024

Since the issue #5164 brought up webfonts once more and the other addon is quite outdated, I started to build a new one.

TODO:

  • fix d.ts file
  • remove Roboto Mono from client.ts and client.css again
  • extend demo to test webfont loading
  • find a working CI test pattern

addons/addon-web-fonts/src/WebFontsAddon.ts Dismissed Show dismissed Hide dismissed
@jerch
Copy link
Member Author

jerch commented Oct 3, 2024

@Tyriar I am not sure, if the integration testing works correctly atm. There are several tests failing, but the CI does still mark them as successful. 🤔

Example: https://github.com/xtermjs/xterm.js/actions/runs/11170843380/job/31054434727?pr=5178#step:10:491

demo/server.js Dismissed Show dismissed Hide dismissed
demo/server.js Dismissed Show dismissed Hide dismissed
@Tyriar
Copy link
Member

Tyriar commented Oct 4, 2024

I am not sure, if the integration testing works correctly atm. There are several tests failing, but the CI does still mark them as successful. 🤔

Ugh, this again ☹️

@Tyriar
Copy link
Member

Tyriar commented Oct 4, 2024

Will handle in #5179

@Tyriar
Copy link
Member

Tyriar commented Oct 4, 2024

Fixed: #5180

@jerch
Copy link
Member Author

jerch commented Oct 4, 2024

I have no clue yet, how to test that in the CI. Maybe it is possible to track the font asset loading somehow with playwright. The changing glyph metrics should be spotable though (my demo fonts are quite different from the usual ones).

@jerch
Copy link
Member Author

jerch commented Oct 4, 2024

@Tyriar Added some CI tests, ready for review.

@jerch jerch requested a review from Tyriar October 4, 2024 15:57
@jerch jerch added this to the 6.0.0 milestone Oct 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants